Skip to content

C++/C#/Java: Shared TaintTrackingImpl.qll#1790

Merged
hvitved merged 2 commits into
github:masterfrom
jbj:tainttracking-cross-language
Aug 22, 2019
Merged

C++/C#/Java: Shared TaintTrackingImpl.qll#1790
hvitved merged 2 commits into
github:masterfrom
jbj:tainttracking-cross-language

Conversation

@jbj

@jbj jbj commented Aug 21, 2019

Copy link
Copy Markdown
Contributor

This file is now identical in all languages. Unifying this file led to the following changes:

  • The documentation spelling fixes and example from the C++ version
    were copied to the other versions and updated.
  • The steps through NonLocalJumpNode from C# were abstracted into a
    globalAdditionalTaintStep predicate that's empty for C++ and Java.
  • The defaultTaintBarrier predicate from Java is now present but empty
    on C++ and C#.
  • The C++ isAdditionalFlowStep predicate on
    TaintTracking::Configuration no longer includes localFlowStep.
    That should avoid some unnecessary tuple copying.

This file is now identical in all languages. Unifying this file led to
the following changes:
- The documentation spelling fixes and example from the C++ version
  were copied to the other versions and updated.
- The steps through `NonLocalJumpNode` from C# were abstracted into a
  `globalAdditionalTaintStep` predicate that's empty for C++ and Java.
- The `defaultTaintBarrier` predicate from Java is now present but empty
  on C++ and C#.
- The C++ `isAdditionalFlowStep` predicate on
  `TaintTracking::Configuration` no longer includes `localFlowStep`.
  That should avoid some unnecessary tuple copying.
@jbj jbj requested review from aschackmull and hvitved August 21, 2019 13:03
@jbj jbj requested review from a team as code owners August 21, 2019 13:03
@jbj

jbj commented Aug 21, 2019

Copy link
Copy Markdown
Contributor Author

I forgot to list

  • The new taint wrappers for barrier guards and in/out barriers are now exposed for C# and C++.
  • The isSanitizerEdge predicate is now deprecated for C# and C++.

final override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
isAdditionalTaintStep(node1, node2) or
localAdditionalTaintStep(node1, node2) or
globalAdditionalTaintStep(node1, node2)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we replace localAdditionalTaintStep(node1, node2) or globalAdditionalTaintStep(node1, node2) with defaultAdditionalTaintStep(node1, node2) in the shared code?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine with me. What do you think, @hvitved ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds fine with me.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@jbj

jbj commented Aug 22, 2019

Copy link
Copy Markdown
Contributor Author

I forgot the C++ IR taint tracking library, both here and in #1757. I'll fix that in a follow-up PR.

@hvitved hvitved merged commit 0801e51 into github:master Aug 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants